Skip to content

Run Hermes adapter through local harness - #35

Open
afrog33k wants to merge 2 commits into
openclaw:mainfrom
afrog33k:main
Open

Run Hermes adapter through local harness#35
afrog33k wants to merge 2 commits into
openclaw:mainfrom
afrog33k:main

Conversation

@afrog33k

@afrog33k afrog33k commented Jul 1, 2026

Copy link
Copy Markdown

Summary

  • Wire registered non-OpenClaw adapters through the generic harness lifecycle instead of rejecting them as known-but-unwired.
  • Add Hermes local adapter configuration from environment and a local verification shim for file/execution checks.
  • Prepend explicit staged-workspace guidance to Hermes MiniSWERunner prompts so local runs inspect the task workspace first.

Validation

  • /Users/reckon/miniconda3/bin/python -m pytest tests/test_harness.py tests/test_hermes_adapter.py -q -> 21 passed
  • /Users/reckon/miniconda3/bin/python -m pytest -q -> 318 passed, 5 skipped
  • Local Hermes rerun: t2-msg-summarize-thread improved from 0.63704 to 1.000 with the workspace-guidance patch.

Notes

  • This keeps OpenClaw behavior intact and routes adapter != "openclaw" through the adapter lifecycle.
  • Memory/session/cron/browser checks still need adapter-aware verification before broad non-file task sweeps.

@afrog33k
afrog33k requested a review from a team as a code owner July 1, 2026 11:36
@clawsweeper

clawsweeper Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codex review: needs changes before merge. Reviewed August 6, 2026, 9:16 AM ET / 13:16 UTC.

ClawSweeper review

What this changes

The PR routes registered non-OpenClaw adapters through ShellBench’s local harness lifecycle and adds workspace-first guidance for Hermes prompts.

Merge readiness

⚠️ Needs maintainer review before merge - 3 items remain

Keep this PR open: current main still rejects Hermes execution, but the unchanged branch scores supported stateful Hermes runs through an unavailable Gateway shim instead of the active adapter.

Priority: P2
Reviewed head: 67ff264005ab6b55679299ba4e7fcd7cbb1276bf

Review scores

Measure Result What it means
Overall readiness 🦐 gold shrimp (3/6) The real local Hermes result is useful evidence, but the P1 state-scoring defect prevents merge readiness.
Proof confidence 🦞 diamond lobster (5/6) Sufficient (live_output): The PR body records an after-fix local Hermes run improving the named task from 0.63704 to 1.000; the remaining blocker is incorrect stateful scoring, not missing proof.
Patch quality 🦐 gold shrimp (3/6) 1 actionable review finding remain.

Verification

Check Result Evidence
Real behavior Verified Sufficient (live_output): The PR body records an after-fix local Hermes run improving the named task from 0.63704 to 1.000; the remaining blocker is incorrect stateful scoring, not missing proof.
Evidence reviewed 6 items Current main has not superseded the capability: Current main explicitly rejects registered adapters that are not OpenClaw for executable harness runs, so Hermes local execution remains unimplemented on main.
PR scores through the wrong verification boundary: The PR creates and runs the adapter but tears it down before calling score_task_run with _LocalVerificationClient; it never invokes adapter.verify_state_query for canonical state queries.
Canonical contract requires adapter state verification: The canonical verifier contract says state queries are resolved by adapter.verify_state_query, separately from workspace file and execution checks.
Findings 1 actionable finding [P1] Route state assertions through the active adapter
Security None None.

How this fits together

ShellBench runs benchmark tasks through an adapter, collects workspace and transcript results, then verifies completion to produce a benchmark score. Local adapters need adapter-specific state verification because they do not use the OpenClaw Gateway RPC interface.

flowchart LR
  Task[Benchmark task] --> Harness[Harness]
  Harness --> Adapter[Hermes adapter]
  Adapter --> Workspace[Task workspace]
  Adapter --> State[Adapter state verification]
  Workspace --> Scoring[Completion scoring]
  State --> Scoring
  Scoring --> Result[Benchmark result]
Loading

Before merge

  • Route state assertions through the active adapter (P1) - _run_single_adapter tears down the adapter and then calls score_task_run with _LocalVerificationClient, without ever invoking adapter.verify_state_query. Hermes AIAgent mode admits memory and cron tasks, so their state assertions are evaluated as unavailable Gateway RPCs instead of Hermes state. This is the unchanged prior P1 blocker.
  • Resolve merge risk (P1) - Supported stateful Hermes tasks can receive incorrect completion scores because their state assertions are evaluated against an unavailable Gateway shim rather than Hermes.
  • Complete next step (P2) - The unchanged P1 blocker has a narrow, source-proven repair at the adapter-to-scorer handoff.

Findings

  • [P1] Route state assertions through the active adapter — clawbench/harness.py:640-651
Agent review details

Security

None.

Review metrics

Metric Value Why it matters
Patch surface 4 files affected; production +247/-10, tests +103/-7 The new 204-line local adapter lifecycle is the primary behavior change and needs coverage beyond a files-only task.

Merge-risk options

Maintainer options:

  1. Preserve adapter-aware state scoring (recommended)
    Pass canonical state-query results from the live adapter into completion scoring and cover a stateful Hermes task before merge.
  2. Narrow local adapter support
    If stateful Hermes tasks are intentionally out of scope, explicitly filter them and document the files-and-execution-only support boundary.
Copy recommended automerge instruction
@clawsweeper automerge

Special instructions:
Route live adapter state-query results into completion scoring before teardown, preserving capability-missing handling and adding a stateful Hermes regression test.

Technical review

Best possible solution:

Carry active-adapter state-query results into completion scoring before teardown, preserving the neutral capability-missing policy and adding a stateful Hermes regression test.

Do we have a high-confidence way to reproduce the issue?

Yes. A Hermes AIAgent-mode task requiring memory or cron reaches the PR’s local adapter path, which admits the capability but scores it using _LocalVerificationClient rather than adapter state verification.

Is this the best way to solve the issue?

No. The local lifecycle is the right direction, but it must feed the adapter’s canonical state-query outcomes into scoring instead of treating them as Gateway RPC checks.

Full review comments:

  • [P1] Route state assertions through the active adapter — clawbench/harness.py:640-651
    _run_single_adapter tears down the adapter and then calls score_task_run with _LocalVerificationClient, without ever invoking adapter.verify_state_query. Hermes AIAgent mode admits memory and cron tasks, so their state assertions are evaluated as unavailable Gateway RPCs instead of Hermes state. This is the unchanged prior P1 blocker.
    Confidence: 0.99

Overall correctness: patch is incorrect
Overall confidence: 0.99

AGENTS.md: not found in the target repository.

Codex review notes: model internal, reasoning high; reviewed against 884dd1bb5511.

Labels

Label justifications:

  • P2: This is a bounded benchmark-validity defect affecting local adapter runs rather than the core OpenClaw execution path.
  • merge-risk: 🚨 other: Merging would permit benchmark results whose supported adapter-state assertions are scored through the wrong verification boundary.
  • rating: 🦐 gold shrimp: Overall readiness is 🦐 gold shrimp; proof is 🦞 diamond lobster and patch quality is 🦐 gold shrimp.
  • status: ⏳ waiting on author: ClawSweeper has contributor-facing work open and is waiting for author action. Sufficient (live_output): The PR body records an after-fix local Hermes run improving the named task from 0.63704 to 1.000; the remaining blocker is incorrect stateful scoring, not missing proof.
  • proof: sufficient: Contributor real behavior proof is sufficient. The PR body records an after-fix local Hermes run improving the named task from 0.63704 to 1.000; the remaining blocker is incorrect stateful scoring, not missing proof.

Evidence

Acceptance criteria:

  • [P1] pytest tests/test_harness.py tests/test_hermes_adapter.py -q.

What I checked:

  • Current main has not superseded the capability: Current main explicitly rejects registered adapters that are not OpenClaw for executable harness runs, so Hermes local execution remains unimplemented on main. (clawbench/harness.py:125, 884dd1bb5511)
  • PR scores through the wrong verification boundary: The PR creates and runs the adapter but tears it down before calling score_task_run with _LocalVerificationClient; it never invokes adapter.verify_state_query for canonical state queries. (clawbench/harness.py:640, 67ff264005ab)
  • Canonical contract requires adapter state verification: The canonical verifier contract says state queries are resolved by adapter.verify_state_query, separately from workspace file and execution checks. (clawbench/canonical/schema.py:165, 884dd1bb5511)
  • Hermes has supported stateful paths: In AIAgent mode Hermes advertises memory and cron capabilities and implements corresponding state-query handling, which the PR bypasses during scoring. (clawbench/adapters/hermes.py:249, 884dd1bb5511)
  • Prior blocker remains at the same PR head: The current PR head is the same commit previously reviewed; its parent diff shows the state-scoring path originates in the first PR commit, while the current head only adds Hermes prompt guidance. (clawbench/harness.py:640, 67ff264005ab)
  • Adapter-contract provenance: The current Hermes state-query implementation was introduced with adapter canonicalization. (clawbench/adapters/hermes.py:598, 56531fbf432b)

Likely related people:

  • scoootscooob: Introduced the canonical adapter and Hermes state-query behavior that this harness path needs to use. (role: introduced adapter verification contract; confidence: high; commits: 56531fbf432b; files: clawbench/adapters/hermes.py, clawbench/adapters/base.py, clawbench/canonical/schema.py)
  • Vincent Koc: Recent current-main commits cover harness runtime and scoring behavior adjacent to this integration. (role: recent harness and scoring contributor; confidence: medium; commits: ed9adf8d8451, fb486a1ed307; files: clawbench/harness.py, clawbench/scorer.py)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

  • Route live adapter state-query results into scoring before teardown.
  • Add a stateful Hermes regression test covering an admitted capability.

Rating scale

Score Internal tier Crab rank Meaning
6/6 S 🦀 challenger crab Exceptional readiness
5/6 A 🦞 diamond lobster Very strong readiness
4/6 B 🐚 platinum hermit Good normal PR; ordinary maintainer review
3/6 C 🦐 gold shrimp Useful, but confidence is limited
2/6 D 🦪 silver shellfish Proof or implementation needs work
1/6 F 🧂 unranked krab Not merge-ready
N/A NA 🌊 off-meta tidepool Rating does not apply

Overall follows the weaker of proof and patch quality.
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics.

Workflow

  • ClawSweeper keeps one durable marker-backed review comment per issue or PR.
  • Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
  • A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
  • PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
  • Maintainers can also comment @clawsweeper review to request a fresh review only.
  • Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
  • Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
  • Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

History

Review history (45 earlier review cycles; latest 8 shown)
  • reviewed 2026-08-05T08:36:40.730Z sha 67ff264 :: needs changes before merge. :: [P1] Route state queries through the active adapter
  • reviewed 2026-08-05T12:07:53.539Z sha 67ff264 :: needs changes before merge. :: [P1] Route state assertions through the live adapter
  • reviewed 2026-08-05T19:41:37.375Z sha 67ff264 :: needs changes before merge. :: [P1] Route state assertions through the live adapter
  • reviewed 2026-08-05T20:51:11.865Z sha 67ff264 :: needs changes before merge. :: [P1] Route state assertions through the active adapter
  • reviewed 2026-08-05T23:39:13.125Z sha 67ff264 :: needs changes before merge. :: [P1] Route state assertions through the active adapter
  • reviewed 2026-08-06T07:18:19.714Z sha 67ff264 :: needs changes before merge. :: [P1] Pass adapter state verification into scoring
  • reviewed 2026-08-06T10:16:42.347Z sha 67ff264 :: found issues before merge. :: [P1] Route state assertions through the active adapter
  • reviewed 2026-08-06T11:57:31.486Z sha 67ff264 :: needs changes before merge. :: [P1] Route state assertions through the active adapter

@clawsweeper clawsweeper Bot added proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 1, 2026
@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. labels Jul 1, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. and removed rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 compatibility 🚨 Merging this PR could break existing users, config, migrations, defaults, or upgrades. labels Jul 29, 2026
@clawsweeper clawsweeper Bot added rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. and removed rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. rating: 🦪 silver shellfish Thin PR readiness signal; proof, validation, or implementation needs work. merge-risk: 🚨 session-state 🚨 Merging this PR could lose, corrupt, stale, or mis-associate session or agent state. merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. labels Aug 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 other 🚨 Merging this PR has meaningful risk outside the owned taxonomy. P2 Normal priority bug or improvement with limited blast radius. proof: sufficient Contributor real behavior proof is sufficient. rating: 🦐 gold shrimp Decent PR readiness signal, but merge confidence is limited. status: ⏳ waiting on author ClawSweeper has contributor-facing work open and is waiting for author action.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant